Skip to content

Add support for Jujutsu conflict markers - #64750

Open
Ryan Fitzgerald (rf-) wants to merge 1 commit into
microsoft:mainfrom
rf-:support-jujutsu-conflicts
Open

Ryan Fitzgerald (rf-) wants to merge 1 commit into
microsoft:mainfrom
rf-:support-jujutsu-conflicts

Conversation

@rf-

@rf- Ryan Fitzgerald (rf-) commented Oct 12, 2026 •

Copy link
Copy Markdown

By default, the Jujutsu VCS represents
conflicts using a different style than Git. The overall conflict area is
still delimited by <<<<<<< and >>>>>>> lines, but the intermediate
sections are separated by %, \, +, and/or - characters instead
of = and |.

We have two basic requirements for handling conflict areas:

  • The conflict markers themselves should be flagged as errors, and the
    compiler should ignore all other text on each marker line.

  • Exactly one version of the conflicted code should be processed by the
    compiler, and any other conflict sections should be ignored. This
    reduces the odds of spurious errors showing up in surrounding code.

With Git conflicts, these goals can be achieved without keeping any
state between markers. Each call to scanConflictMarkerTrivia() handles
one marker line, and the call for each intermediate marker also consumes
other file content until the next intermediate marker, while leaving the first
section after <<<<<<< to be scanned normally.

Jujutsu conflicts are trickier. The <<<<<<< line is followed by
another marker line, not the actual content, and there may be multiple
"snapshot" sections marked by +++++++ lines that contain content from
different commits. We want to scan one of these sections but omit any
others, so we need to keep a flag tracking whether we've allowed one to
be scanned yet.

Another subtlety here is that if the file contains any lines that look
like normal-length conflict markers, Jujutsu will use longer markers to
disambiguate. Handling this correctly requires us to track the length of
the markers in the current conflicted section.

Points of interest:

  • There's a private couldStartTrivia() function that I would have had
    to update to keep in sync with skipTrivia(), but it seems to just be
    dead code. This commit deletes it.

  • The old code is picky about whitespace after the initial marker
    string, requiring a space character after anything but =. This is
    simplified now so that any marker string can be followed by either a
    space or a newline.

This adds a test case for each of the scenarios described in the Jujutsu
docs (default style, snapshot style, longer markers, and missing
terminating newline) and one for CRLF line endings.

Copilot AI balanced review requested due to automatic review settings October 12, 2026 18:56
@typescript-automation typescript-automation Bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Oct 12, 2026
@typescript-automation

Copy link
Copy Markdown
Contributor

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The scanner changes are consistent with existing conflict-marker handling and comprehensively covered by compiler baselines.

0 open findings

What changed in this PR

Adds native compiler support for Jujutsu-style conflict markers.

Changes:

  • Recognizes %, \, and + conflict separators.
  • Extends conflict-trivia scanning and skipping.
  • Adds compiler coverage and expected baselines.
File Description
tsc/​internal/​scanner/​scanner.go Recognizes and skips Jujutsu conflict sections.
tsc/​testdata/​tests/​cases/​compiler/​conflictMarkerJujutsuTrivia1.ts Exercises all Jujutsu marker forms.
tsc/​testdata/​baselines/​reference/​compiler/​conflictMarkerJujutsuTrivia1.errors.txt Verifies TS1185 diagnostics.
tsc/​testdata/​baselines/​reference/​compiler/​conflictMarkerJujutsuTrivia1.js Verifies conflicted code is omitted from emit.
tsc/​testdata/​baselines/​reference/​compiler/​conflictMarkerJujutsuTrivia1.symbols Verifies unaffected symbol discovery.
tsc/​testdata/​baselines/​reference/​compiler/​conflictMarkerJujutsuTrivia1.types Verifies unaffected type discovery.

🧠 Review effort: Balanced


💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

@rf-

Copy link
Copy Markdown
Author

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

Is this mandatory or just a nice-to-have? I'm happy to do it if any maintainers feel like it's necessary/helpful in this case, but not sure if it's needed here.

@rf-

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

@jakebailey

Copy link
Copy Markdown
Member

TypeScript Bot (@typescript-bot) perf test this faster

@typescript-automation

typescript-automation Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Starting jobs; this comment will be updated as builds start and complete.

Command Status Results
perf test this faster ✅ Started 👀 Results

@typescript-automation

Copy link
Copy Markdown
Contributor

Jake Bailey (@jakebailey)
The results of the perf run you requested are in!

Here they are:

tsc

Comparison Report - baseline..pr
Metric baseline pr Delta Best Worst p-value
Compiler-Unions - native
Errors 41 41 ~ ~ ~ p=1.000 n=12
Symbols 115,276 115,276 ~ ~ ~ p=1.000 n=12
Types 79,613 79,613 ~ ~ ~ p=1.000 n=12
Memory Used 133,837k (± 0.03%) 133,895k (± 0.06%) ~ 133,695k 134,121k p=0.561 n=12
Memory Allocs 2,190,072 (± 0.01%) 2,190,044 (± 0.00%) ~ 2,189,785 2,190,284 p=0.843 n=12
Config Time 0.000s 0.000s ~ ~ ~ p=1.000 n=12
Parse Time 0.049s (± 3.21%) 0.050s (± 3.32%) ~ 0.046s 0.054s p=0.243 n=12
Bind Time 0.017s (±12.00%) 0.015s (± 9.76%) ~ 0.012s 0.019s p=0.413 n=12
Check Time 0.596s (± 0.47%) 0.599s (± 0.51%) ~ 0.593s 0.607s p=0.310 n=12
Emit Time 0.323s (± 2.09%) 0.328s (± 2.07%) ~ 0.316s 0.349s p=0.324 n=12
Total Time 0.992s (± 0.56%) 1.000s (± 0.71%) ~ 0.987s 1.023s p=0.066 n=12
angular-1 - native
Errors 3 3 ~ ~ ~ p=1.000 n=12
Symbols 845,785 (± 0.07%) 845,600 (± 0.09%) ~ 843,142 847,125 p=0.887 n=12
Types 248,125 (± 0.00%) 248,125 (± 0.00%) ~ 248,122 248,127 p=0.877 n=12
Memory Used 750,371k (± 0.01%) 750,514k (± 0.02%) ~ 750,107k 750,845k p=0.101 n=12
Memory Allocs 12,916,135 (± 0.05%) 12,925,370 (± 0.11%) ~ 12,910,905 12,983,785 p=0.101 n=12
Config Time 0.020s (± 1.59%) 0.020s (± 1.60%) ~ 0.020s 0.021s p=1.000 n=12
Parse Time 0.320s (± 2.02%) 0.320s (± 2.62%) ~ 0.286s 0.338s p=0.921 n=12
Bind Time 0.072s (± 1.76%) 0.079s (±12.40%) ~ 0.068s 0.112s p=0.712 n=12
Check Time 0s 0s ~ ~ ~ p=1.000 n=12
Emit Time 1.870s (± 0.91%) 1.869s (± 1.12%) ~ 1.820s 1.937s p=0.765 n=12
Total Time 2.298s (± 0.81%) 2.304s (± 1.18%) ~ 2.248s 2.403s p=0.723 n=12
mui-docs - native
Errors 11,405 11,405 ~ ~ ~ p=1.000 n=12
Symbols 4,359,614 4,359,614 ~ ~ ~ p=1.000 n=12
Types 1,376,379 1,376,379 ~ ~ ~ p=1.000 n=12
Memory Used 2,813,817k (± 0.05%) 2,813,443k (± 0.05%) ~ 2,810,170k 2,817,135k p=0.671 n=12
Memory Allocs 32,814,088 (± 0.05%) 32,810,969 (± 0.04%) ~ 32,783,251 32,847,888 p=0.843 n=12
Config Time 0.017s 0.017s ~ ~ ~ p=1.000 n=12
Parse Time 0.503s (± 3.64%) 0.509s (± 2.29%) ~ 0.466s 0.534s p=0.744 n=12
Bind Time 0.002s 0.002s ~ ~ ~ p=1.000 n=12
Check Time 8.637s (± 0.54%) 8.603s (± 0.59%) ~ 8.494s 8.739s p=0.247 n=12
Emit Time 0.453s (± 9.12%) 0.450s (± 9.42%) ~ 0.417s 0.600s p=0.068 n=12
Total Time 10.383s (± 0.62%) 10.340s (± 0.63%) ~ 10.210s 10.508s p=0.242 n=12
strada-build-src - native
Errors 0 0 ~ ~ ~ p=1.000 n=12
Symbols 1,390,229 1,390,229 ~ ~ ~ p=1.000 n=12
Types 441,498 441,498 ~ ~ ~ p=1.000 n=12
Memory Used 1,632,907k (± 1.04%) 1,641,907k (± 0.99%) ~ 1,604,427k 1,685,990k p=0.514 n=12
Memory Allocs 92,075,010 (± 0.19%) 92,145,227 (± 0.19%) ~ 91,492,621 92,399,336 p=0.347 n=12
Config Time 0.004s (±14.34%) 0.003s (±16.58%) ~ 0.002s 0.005s p=0.966 n=12
Parse Time 0.196s (± 3.56%) 0.204s (± 4.16%) ~ 0.177s 0.222s p=0.110 n=12
Bind Time 0.000s 0.000s ~ ~ ~ p=1.000 n=12
Check Time 1.795s (± 0.71%) 1.801s (± 0.89%) ~ 1.775s 1.870s p=0.831 n=12
Emit Time 0.300s (± 4.30%) 0.304s (± 3.13%) ~ 0.272s 0.327s p=0.788 n=12
Total Time 23.283s (± 0.42%) 23.474s (± 0.56%) +0.191s (+ 0.82%) 23.141s 23.795s p=0.033 n=12
strada-compiler - native
Errors 0 0 ~ ~ ~ p=1.000 n=12
Symbols 334,953 334,953 ~ ~ ~ p=1.000 n=12
Types 197,616 197,616 ~ ~ ~ p=1.000 n=12
Memory Used 297,324k (± 0.02%) 297,364k (± 0.03%) ~ 297,077k 297,624k p=0.347 n=12
Memory Allocs 4,621,212 (± 0.01%) 4,621,474 (± 0.01%) ~ 4,619,777 4,623,292 p=0.799 n=12
Config Time 0.001s 0.001s ~ ~ ~ p=1.000 n=12
Parse Time 0.115s (± 5.11%) 0.117s (± 3.22%) ~ 0.107s 0.127s p=0.434 n=12
Bind Time 0.000s 0.000s ~ ~ ~ p=1.000 n=12
Check Time 1.038s (± 0.63%) 1.038s (± 0.38%) ~ 1.030s 1.052s p=0.876 n=12
Emit Time 0.142s (± 8.58%) 0.150s (± 6.92%) ~ 0.122s 0.172s p=0.401 n=12
Total Time 1.349s (± 0.81%) 1.359s (± 1.05%) ~ 1.322s 1.384s p=0.235 n=12
ts-pre-modules - native
Errors 87 87 ~ ~ ~ p=1.000 n=12
Symbols 303,834 303,834 ~ ~ ~ p=1.000 n=12
Types 181,663 181,663 ~ ~ ~ p=1.000 n=12
Memory Used 258,185k (± 0.02%) 258,136k (± 0.01%) ~ 258,063k 258,189k p=0.060 n=12
Memory Allocs 1,613,752 (± 0.01%) 1,613,659 (± 0.01%) ~ 1,613,156 1,613,999 p=0.660 n=12
Config Time 0.000s 0.000s ~ ~ ~ p=1.000 n=12
Parse Time 0.099s (± 4.15%) 0.100s (± 3.84%) ~ 0.092s 0.113s p=0.876 n=12
Bind Time 0.039s (±10.55%) 0.040s (±14.96%) ~ 0.027s 0.059s p=0.921 n=12
Check Time 0.813s (± 0.98%) 0.817s (± 0.77%) ~ 0.805s 0.837s p=0.401 n=12
Emit Time 0.000s 0.000s ~ ~ ~ p=1.000 n=12
Total Time 0.965s (± 1.01%) 0.971s (± 0.64%) ~ 0.956s 0.986s p=0.235 n=12
vscode - native
Errors 395 395 ~ ~ ~ p=1.000 n=12
Symbols 11,287,880 11,287,880 ~ ~ ~ p=1.000 n=12
Types 3,740,417 3,740,417 ~ ~ ~ p=1.000 n=12
Memory Used 6,523,082k (± 0.01%) 6,522,944k (± 0.02%) ~ 6,521,326k 6,527,012k p=0.401 n=12
Memory Allocs 55,464,608 (± 0.01%) 55,468,118 (± 0.02%) ~ 55,453,947 55,512,470 p=0.671 n=12
Config Time 0.075s (± 0.33%) 0.075s (± 0.56%) ~ 0.074s 0.076s p=1.000 n=12
Parse Time 1.869s (± 3.39%) 1.847s (± 3.64%) ~ 1.723s 2.011s p=0.641 n=12
Bind Time 0.585s (±21.42%) 0.583s (±17.42%) ~ 0.442s 0.869s p=0.966 n=12
Check Time 14.062s (± 1.04%) 14.060s (± 1.22%) ~ 13.651s 14.312s p=0.601 n=12
Emit Time 4.769s (±11.22%) 4.815s (±10.58%) ~ 4.119s 6.099s p=0.799 n=12
Total Time 21.443s (± 2.19%) 21.468s (± 1.84%) ~ 20.913s 22.507s p=0.744 n=12
webpack - native
Errors 584 584 ~ ~ ~ p=1.000 n=12
Symbols 1,321,840 1,321,840 ~ ~ ~ p=1.000 n=12
Types 624,651 624,651 ~ ~ ~ p=1.000 n=12
Memory Used 957,970k (± 0.02%) 957,754k (± 0.01%) -216k (- 0.02%) 957,426k 958,212k p=0.018 n=12
Memory Allocs 6,693,193 (± 0.04%) 6,691,530 (± 0.02%) ~ 6,689,804 6,696,869 p=0.378 n=12
Config Time 0.009s 0.009s ~ ~ ~ p=1.000 n=12
Parse Time 0.292s (± 4.05%) 0.294s (± 3.16%) ~ 0.269s 0.319s p=0.744 n=12
Bind Time 0.097s (±19.14%) 0.083s (±15.64%) ~ 0.063s 0.124s p=0.369 n=12
Check Time 1.878s (± 1.07%) 1.891s (± 0.89%) ~ 1.846s 1.927s p=0.433 n=12
Emit Time 0.000s 0.000s ~ ~ ~ p=1.000 n=12
Total Time 2.312s (± 0.65%) 2.313s (± 0.71%) ~ 2.269s 2.352s p=0.876 n=12
xstate-main - native
Errors 0 0 ~ ~ ~ p=1.000 n=12
Symbols 1,061,928 1,061,928 ~ ~ ~ p=1.000 n=12
Types 389,645 389,645 ~ ~ ~ p=1.000 n=12
Memory Used 554,311k (± 0.02%) 554,329k (± 0.02%) ~ 554,140k 554,744k p=0.945 n=12
Memory Allocs 4,881,429 (± 0.04%) 4,882,938 (± 0.09%) ~ 4,873,521 4,893,356 p=0.514 n=12
Config Time 0.003s 0.003s ~ ~ ~ p=1.000 n=12
Parse Time 0.144s (± 3.79%) 0.149s (± 4.58%) ~ 0.135s 0.169s p=0.399 n=12
Bind Time 0.045s (±21.00%) 0.046s (±15.73%) ~ 0.033s 0.065s p=0.575 n=12
Check Time 1.129s (± 0.80%) 1.138s (± 1.00%) ~ 1.113s 1.169s p=0.155 n=12
Emit Time 0.000s 0.000s ~ ~ ~ p=1.000 n=12
Total Time 1.329s (± 0.73%) 1.346s (± 0.98%) +0.017s (+ 1.26%) 1.310s 1.372s p=0.027 n=12
System info unknown
Hosts
  • native
Scenarios
  • Compiler-Unions - native
  • angular-1 - native
  • mui-docs - native
  • strada-build-src - native
  • strada-compiler - native
  • ts-pre-modules - native
  • vscode - native
  • webpack - native
  • xstate-main - native
Benchmark Name Iterations
Current pr 12
Baseline baseline 12

Developer Information:

Download Benchmarks

Comment thread tsc/internal/scanner/scanner.go Outdated
Comment thread tsc/internal/scanner/scanner.go Outdated
@RyanCavanaugh

Copy link
Copy Markdown
Member

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

Is this mandatory or just a nice-to-have? I'm happy to do it if any maintainers feel like it's necessary/helpful in this case, but not sure if it's needed here.

An issue would be a better place to sort out the comments that are currently being discussed in the code review (e.g. what are the actual observed forms of the conflict marker?).

@rf-

Copy link
Copy Markdown
Author

Good points, thanks Jake Bailey (@jakebailey)! Apologies for not covering all of the possible formats, it definitely makes sense to do them all at once. I'll open an issue to ref in the PR.

@rf-

Copy link
Copy Markdown
Author

Coming back to this, one thing I'm confused about is the couldStartTrivia function. It seems like it's been dead code in Go since it was added. I'm wondering if maybe it was only retained in the TypeScript codebase because it's exported, and then in Go it's just vestigial and should be removed?

@rf-
Ryan Fitzgerald (rf-) marked this pull request as draft October 17, 2026 17:50
@jakebailey

Copy link
Copy Markdown
Member

I think we can probably just do the simple easy version of this and it be sufficient; this is a best effort recovery after all so I think that's ok.

@rf-

Copy link
Copy Markdown
Author

Jake Bailey (@jakebailey) I ended up feeling like it was worth it to just bite the bullet and do it correctly, since parsing one side of the conflict instead of dropping everything makes a big difference to the editor experience. I just pushed an update that should hopefully address everything that's come up 🙏

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Bare markers at EOF can panic, and CRLF conflicts discard every snapshot instead of preserving one version.

2 open issues

Review effort: Balanced

Comment thread tsc/internal/scanner/scanner.go
Comment thread tsc/internal/scanner/scanner.go
@rf-
Ryan Fitzgerald (rf-) marked this pull request as draft October 26, 2026 00:48
@rf-
Ryan Fitzgerald (rf-) requested a balanced review from Copilot October 26, 2026 01:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Shared conflict-state mutation breaks scanner rewind behavior, and consecutive skipped snapshots can lose required marker diagnostics.

2 open issues
2 resolved since last review

Review effort: Balanced

Comment thread tsc/internal/scanner/scanner.go Outdated
Comment thread tsc/internal/scanner/scanner.go Outdated
By default, the [Jujutsu](https://github.com/jj-vcs/jj) VCS represents
conflicts using a different style than Git. The overall conflict area is
still delimited by `<<<<<<<` and `>>>>>>>` lines, but the intermediate
sections are separated by `%`, `\`, `+`, and/or `-` characters instead
of `=` and `|`.

We have two basic requirements for handling conflict areas:

- The conflict markers themselves should be flagged as errors, and the
  compiler should ignore all other text on each marker line.

- Exactly one version of the conflicted code should be processed by the
  compiler, and any other conflict sections should be ignored. This
  reduces the odds of spurious errors showing up in surrounding code.

With Git conflicts, these goals can be achieved without keeping any
state between markers. Each call to `scanConflictMarkerTrivia()` handles
one marker line, and the call for each intermediate marker also consumes
the content until the next intermediate marker, while leaving the first
section after `<<<<<<<` to be scanned normally.

Jujutsu conflicts are trickier. The `<<<<<<<` line is followed by
another marker line, not the actual content, and there may be multiple
"snapshot" sections marked by `+++++++` lines that contain content from
different commits. We want to scan one of these sections but omit any
others, so we need to keep a flag tracking whether we've allowed one to
be scanned yet.

Another subtlety here is that if the file contains any lines that look
like normal-length conflict markers, Jujutsu will use longer markers to
disambiguate. Handling this correctly requires us to track the length of
the markers in the current conflicted section.

Points of interest:

- There's a private `couldStartTrivia()` function that I would have had
  to update to keep in sync with `skipTrivia()`, but it seems to just be
  dead code.  This commit deletes it.

- The old code is picky about whitespace after the initial marker
  string, requiring a space character after anything but `=`. This is
  simplified now so that any marker string can be followed by either a
  space or a newline.

This adds a test case for each of the scenarios described in the Jujutsu
docs (default style, snapshot style, longer markers, and missing
terminating newline) and one for CRLF line endings.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Mid-conflict trivia scanning loses marker-width context, and JSX scanning can process discarded conflict sections.

2 open issues
2 resolved since last review

Review effort: Balanced

Comment on lines +1321 to +1322
if isConflictMarkerTrivia(s.text, s.pos, s.conflictState) {
s.pos, s.conflictState = scanConflictMarkerTrivia(s.text, s.pos, s.conflictState, s.errorAt)

@rf- Ryan Fitzgerald (rf-) Oct 26, 2026 •

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this related to this PR? If the existing JSX scanner doesn't handle intermediate markers properly in general, then fixing that doesn't seem in scope here 🤔

@rf- Ryan Fitzgerald (rf-) Oct 26, 2026 •

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, conflicts inside JSX are not working well on main now from either an error or codegen perspective. I think fixing that is a bit above my pay grade, but I don't think this is making it any worse—the current code already doesn't handle ||| or === markers at all, so adding the jj markers doesn't matter too much.


// Now that we've established we don't want to scan this section, consume
// everything up to the start of the next marker.
for pos < length {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, OK. This seems very edge casey IIUC, and it's still not solving the broader issue that SkipTriviaEx doesn't know the expected marker length if it starts halfway through a conflict area, but it seems reasonable to initialize the state if we start at an intermediate marker.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually the hard part here is the test case—SkipTriviaEx itself doesn't have tests, and I don't know what call site would make sense to try to test this from. It would probably make sense to use one where this situation is likely to come up and would have a meaningful impact, but I'm not sure if that exists.

@rf-
Ryan Fitzgerald (rf-) marked this pull request as ready for review October 26, 2026 04:46

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

For Uncommitted Bug PR for untriaged, rejected, closed or missing bug

Projects

Status: Not started

Development

Successfully merging this pull request may close these issues.

4 participants